Skip to content

feat(runtime): add Plugin Platform foundation - #3729

Open
xxhZs wants to merge 5 commits into
apache:mainfrom
xxhZs:feat/plugin-platform-foundation
Open

feat(runtime): add Plugin Platform foundation#3729
xxhZs wants to merge 5 commits into
apache:mainfrom
xxhZs:feat/plugin-platform-foundation

Conversation

@xxhZs

@xxhZs xxhZs commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR adds the Runtime Host Plugin Platform foundation from trusted Package ingestion through durable desired state and Context/Fiber convergence.

canonical Package bytes + Composition authority
  (ordered Package IDs/layers + User overlays)
                    |
                    v
           derived Desired Entry Tree
                    |
                    v
             Context/Fiber Runtime

The durable authority remains the canonical Package bytes plus the Composition ledger. This PR does not restore the removed Package Revision/Version/Binding lifecycle. Package queries expose a computed contentDigest only as an inspection fact; it is not persisted as revision state.

Package and Composition foundation

  • validates trusted Package directories and .maka-extension bundles
  • stores canonical Package bytes with crash-safe install/replacement recovery
  • applies ordered Package Composition layers, followed by User overlays
  • derives the Desired Entry Tree without persisting a duplicate resolved tree
  • loads immutable executable generations for active Package lifetimes
  • supports install, uninstall, reload, export, overlay apply, bounded query, and explicit reconcile through the CLI
  • requires cross-Package structural dependencies to be declared and verifies them against the projected tree
  • exposes manifest and structural dependency edges, including reverse requiredBy relationships

Explicit lifecycle and mutation outcomes

The Platform now enforces one-shot recovery and exposes independent control and convergence state:

phase:       new -> recovering -> ready/degraded/fenced -> draining -> closed
convergence: unknown | converged | diverged
  • mutations and queries are rejected before recovery; a second recovery is rejected
  • known authority commits are never erased when Runtime convergence later fails
  • mutation receipts identify the authority epoch, committed durability, convergence, cleanup state, and bounded failures
  • status distinguishes physical Packages, layered Packages, desired Entries, and live Entries
  • status exposes a bounded fence diagnostic without leaking arbitrary causes
  • divergence schedules bounded exponential-backoff reconciliation and may also be repaired with plugin platform reconcile
  • Package cleanup after a committed uninstall reports cleanup: pending rather than pretending the mutation was rejected

Consistency boundaries

  • the Platform privately owns both Stores and the Package Loader; lower-level mutation surfaces are no longer public exports
  • all online and recovery Desired Tree derivation uses one projector with strict and fail-open policies
  • Package replacement uses break-before-make for existing Entries, avoiding single-provider Service collisions
  • query cursors are opaque and bound to a stable snapshot; a mutation between pages returns stale_cursor
  • Plugin Platform operations are local-owner-only; remote owners cannot admit them

Scope

This PR stops at Package -> Desired Entry -> Context/Fiber convergence. It does not add Desktop management UI, Agent-facing management tools, Marketplace integration, concrete Tool/UI/Hook/Event contribution consumers, or the removed Package revision lifecycle.

Verification

  • complete workspace build passed
  • Runtime Host: 1390 passed, 9 skipped, 0 failed
  • Plugin Platform focused suite: 35 passed, 0 failed
  • Runtime Host CLI focused suite: 7 passed, 0 failed
  • Biome format/lint and git diff --check passed

@xxhZs
xxhZs force-pushed the feat/plugin-platform-foundation branch 2 times, most recently from d399570 to 71803a2 Compare August 24, 2026 17:39
@xxhZs
xxhZs marked this pull request as ready for review August 25, 2026 02:21

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Install rollback can silently fail while the new package appears committed
plugin-package-store.ts:193 reports persistence_failed when both new target and .previous-* exist, but recovery later deletes the old package and confirms the new executable. Must report commit_outcome_unknown and fence until the directory is durable.

[P2] Fence check is outside the serialized queue
Two same-tick mutations both pass #assertMutable() before the fence is set; the second queued mutation still executes and overwrites the ambiguous state. Must re-check inside the serialized callback.

[P2] Output size checked after commit
Desired tree size (512 KiB) is only checked on return. A store within the 2 MiB input limit can accumulate entries that make apply/query responses exceed the limit after commit, leaving the commit durable but the caller with internal_failure.

[P3] Foundation without concrete consumer
~4.7k lines introduce client/UI/config APIs with no consumer yet; many helpers are unused. Consider delivering as minimal vertical slices rather than a large foundation.

Checks on 71803a267d are not green due to local-only surface enumeration mismatch — not green.

简体中文存在三项持久化/并发/输出阻断与一项熵增观察。

@xxhZs
xxhZs force-pushed the feat/plugin-platform-foundation branch 3 times, most recently from e535c71 to 580a099 Compare August 25, 2026 14:29
@xxhZs xxhZs changed the title feat(runtime): add plugin platform foundation feat(runtime): install Plugin packages through Runtime Host Aug 25, 2026
@xxhZs

xxhZs commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all four findings in 580a09982.

  • Install rollback outcome: Package rollback no longer swallows removal, restore, or directory-sync failures. Any rollback that cannot confirm a durable directory state returns commit_outcome_unknown, retains recoverable transaction remnants, and immediately fences the Plugin Platform.
  • Fence admission: mutable state is re-checked inside the serialized callback. A same-tick operation that queued before another operation established a fence can no longer execute afterward. Added a regression that holds the first commit, queues the second mutation, establishes an unknown outcome, and verifies the second never reaches persistence.
  • Post-commit output limit: removed the speculative full-state plugin.composition.apply and plugin.platform.query responses. The public slice now exposes only fixed, bounded install/uninstall results, so a successful durable commit cannot fail later while encoding a materialized Desired Tree or Inspection projection.
  • Concrete consumer and scope: added maka runtime-host plugin install|uninstall as the end-to-end consumer. Removed the unused public reload, export, generic composition apply, full query, Runtime projection/digest, and bundle-export surfaces. The public protocol is now two lifecycle operations instead of six, and the PR changed from +4859/-64 to +4528/-78.

Verification: full workspace build, Biome, ASF headers, and diff check passed; Runtime 3037/3037 active tests and CLI 455/455 passed; Plugin Platform + CLI focused coverage is 25/25. Runtime Host passed 1195/1196 in the parallel run; the sole unrelated shared registration-directory race passed 3/3 isolated reruns.

@xxhZs
xxhZs force-pushed the feat/plugin-platform-foundation branch from 580a099 to e535c71 Compare August 25, 2026 14:58
@xxhZs xxhZs changed the title feat(runtime): install Plugin packages through Runtime Host feat(runtime): add Plugin Platform foundation Aug 25, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on e535c712e3:

[P1] Package replacement publishes before authority commit

plugin-package-store.ts:146-202 renames new bytes to canonical before commit(); plugin-platform.ts:154-189 reloads Entries before Composition authority commits. On crash between publish/authority commit, restart replays old packageLayers with new patch — mixed durable authority. Also 193-199 silently demotes restore/sync failures to persistence_failed and no longer fences unknown canonical.

Fix: atomize publish with authority commit before converging runtime; restore fencing for unknown persistence.

[P1] Queued mutation bypasses unknown-outcome drain

Removal of #assertMutable() inside queue allows a queued operation to execute after a prior commit_outcome_unknown drain/poison, overwriting the prior authority fact with stale in-memory state.

Fix: re-check #assertMutable() at dequeue time and keep commit_outcome_unknown fencing; restore regression test.

[P2] Uninstall leaves authority mutated on package_in_use failure

plugin-platform.ts:247-260 deletes layer and converges before checking surviving overlay still uses package — default layer already removed when error returned.

[P2] Response codec limit below durable limit

Composition allows 2 MiB but query/apply response capped 512 KiB — accumulated authority can outgrow response and become un-decodable after commit.

Also hard Standards breaches on phase ordering and close aggregation require fix.

Checks on e535c712e3 are test: IN_PROGRESS — not green.

简体中文存在包替换与排队栅栏等阻塞。

@xxhZs
xxhZs force-pushed the feat/plugin-platform-foundation branch from e535c71 to 85b5aac Compare August 25, 2026 15:42
@xxhZs

xxhZs commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Reworked the current head against the latest review in 85b5aac09:

  • Package replacement now uses a durable generation journal. Candidate bytes are validated first; canonical bytes and Composition authority commit before Runtime convergence. Recovery restores the previous Package at the base generation and retains the new Package at or beyond the committed generation.
  • Unknown Package/Composition outcomes fence the platform, and every queued mutation re-checks the fence when dequeued.
  • Uninstall plans and validates the candidate authority before committing, so package_in_use leaves layers and Desired state unchanged.
  • plugin.composition.apply now returns only the committed generation. plugin.platform.query remains available through bounded status/packages/entries/failures views with byte-bounded cursor pages.
  • All six operations now have concrete maka runtime-host plugin ... CLI consumers; Bundle export remains supported. The unused Runtime digest/projection contract was removed.
  • Close attempts both Runtime composition and generation-loader cleanup and aggregates failures.

Added crash-boundary, queued-fence, uninstall, response-size, close-aggregation, and CLI consumer regressions. Full Runtime (3037), Runtime Host (1198), and CLI (455) suites pass, along with full build, Biome, ASF, Windows inventory, and diff checks.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Third review pass on 85b5aac09. All four findings from the previous round are genuinely fixed — I verified each: the fence now re-checks at the serialized dequeue point, replacement journals base→next, both package_in_use checks moved ahead of the authority commit, and paging bounds the response. Bundle import defenses (safePath, sha256, symlink rejection, open(...,'wx')) are sound; I could not construct an escape.

This round found four P1s. Details are inline; two mechanisms span files the diff does not touch, so they are here.

The remote-owner grant is the one I would look at first. operations.ts adds all six operations to REMOTE_OWNER_OPERATION_GRANTS. Four of them (plugin.composition.apply, plugin.package.uninstall, plugin.package.reload, plugin.platform.query) are plain defineOperation, so canUseHostPaths: false does not gate them. runtime-host-access-command.ts:251 — unchanged by this PR — builds every --preset credential from that whole list. A preset-issued remote credential can therefore activate a package into any root, remove or disable any Entry, and uninstall packages outright, all durably. "Packages are trusted code" is the accepted premise of this PR, but it governs what the code may do, not which principal decides when and where it runs.

The provide collision needs plugin-kernel.ts to see: #label() returns a kernel-global symbol when no isolate mapping exists, and provide() throws if that label is already in #kernel.services. Every make-before-break path stages the new Fiber while the old one still holds the label, so move and reload fail for any Entry calling ctx.provide. isolate does not help — it makes the service private to the subtree. Graded P2 only because this PR ships no contribution consumers yet, so no service-providing plugin exists in tree; it must be fixed before one does.

Three P3s that have no line in the diff to attach to: validatePluginRootId accepts any non-empty session: scopeId, so __proto__ throws a TypeError from siblings.splice instead of being cleanly rejected; decodeScalarRecord/decodeIsolate silently drop prototype-named keys rather than rejecting them; and validateExtensionConfiguration reads input[key] through the prototype chain, so a property named toString/constructor/valueOf makes the package permanently unconfigurable with an error that blames the operator.

Simplification — not blocking, but this is a new 5.6k-line subsystem and worth doing before it grows consumers

Dead on arrival: MakaCompositionLoader.restoreComposition (no consumer, not even a test), walkLive, PluginPackageStore.list(), PluginPackageStore.install() (only caller is a test, and it hardcodes publish(0, 1)), and the .previous-/.staging-/.rejected- branches in recover() — no production path writes a top-level directory with those prefixes.

Duplicate authorities: extension-bundle.ts and plugin-package-store.ts carry near-identical MAX_FILES/MAX_FILE_BYTES/16 MiB limits plus line-for-line copies of safePath and collect; extension-package-manifest.ts:138 holds a third copy of the path predicate that omits the posix.normalize check, so the manifest accepts paths the store rejects. PluginPackageLoaderError + translate() is an identity relabeling — the coordinator's two mapping blocks are character-for-character the same policy. #draining is set alongside #poisoned at all five fence sites, and #poisoned alone already fences; only beginDrain() needs it.

简体中文

第三轮,针对 85b5aac09。上一轮四条都真修好了,逐条验过。本轮四条 P1,详情在行内。

最该先看的是远程授权:六个操作全进 REMOTE_OWNER_OPERATION_GRANTS,其中四个不受 canUseHostPaths 约束,而 preset 从整张表签发凭据。“包是可信代码”管的是代码能做什么,不是哪个主体决定它何时何地运行。

另外两条 P1 在包存储的崩溃恢复上:rollback 中途被杀会导致重启时删掉已恢复的旧包;journal 缺失被当成损坏,导致整个插件子系统跨重启永久围栏。这两条所在的 #recoverInstall 目前测试覆盖为零。

Reviewed with help from Claude.

'plan.query',
'plan.turn.start',
'plugin.composition.apply',
'plugin.package.export',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] All six operations land in REMOTE_OWNER_OPERATION_GRANTS, but only install and export are defineHostPathOperation. apply, uninstall, reload and query are plain defineOperation, so a remote credential with canUseHostPaths: false still gets them — and presets are built from this entire list. That lets a remote principal durably rewrite Host plugin authority. Was this deliberate? Nothing in the diff says so. Suggest local-owner-only for the foundation, or at minimum dropping the mutating four from the preset.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for flagging this. I rechecked it against the existing access contract and do not think this is an authorization bypass. remote_owner is the controlling remote Client principal, not a capability provider; the preset already deliberately grants durable owner mutations such as credential.vault.*, runtime.policy.mutate, project.catalog.mutate, session.remove, and skill.catalog.mutate. canUseHostPaths is narrowly defined as whether a Client may name a Host path in an operation, rather than as a general mutation-authority bit (see runtime-host-architecture.md, Host path authority). Accordingly, install/export use defineHostPathOperation because they accept sourcePath/targetPath; apply/uninstall/reload/query carry no Host-path input. Query is read-only and reload changes only the live projection; apply/uninstall are durable owner operations, consistent with the existing remote-owner model. Their addition to the fail-closed REMOTE_OWNER_OPERATION_GRANTS list is deliberate. Making Plugin Platform local-owner-only would be a new product policy, not a missing canUseHostPaths gate in this PR, so I am leaving these grants unchanged.

const candidateExists = await exists(candidate);
const targetExists = await exists(target);
const previousExists = await exists(previous);
if (authorityGeneration === transaction.baseGeneration) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] This branch infers "the directory at target is the candidate's bytes" from !candidateExists && targetExists, but that is also true after a partial rollback. rollbackPublishedInstall renames target→rejected, then previous→target, then rejected→candidate, with no sync between. Kill the process after step 2: target holds the restored good package, the transaction dir holds transaction.json + rejected, authority is still at baseGeneration. Recovery then rms the restored package and has no previous to put back. Rollback is the ordinary failed-upgrade path, so this is one crash away.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5071441. Recovery now recognizes that a base-generation transaction with rejected present has entered rollback: target is the restored previous Package and is retained instead of deleted. The production-shaped crash matrix covers the publish and rollback rename boundaries for upgrades and fresh installs.

}

async function readTransaction(root: string): Promise<PackageInstallTransaction> {
let value: unknown;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] readTransaction catches everything from readFile, so a missing transaction.json is treated exactly like a corrupt one. #recoverInstall throws before its rm(transactionRoot), recover() catches into #poisoned, and #poisoned is never cleared anywhere — so every subsequent start re-fences and only manual filesystem surgery recovers.

The window is large: prepareInstall creates the transaction directory but writeTransaction only runs inside publish(), so it spans writing up to 256 files, the cp, and await import() of plugin code. commit()'s rm -rf unlinks children first, so a successful install can produce this state too.

ENOENT is provably safe to discard — the journal is written and fsynced before the first rename, so no journal means publish() never ran.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5071441. readTransaction now distinguishes ENOENT from a corrupt journal. Because the journal is durably written before the first publication rename, a journal-less .install-* directory is discarded as a safe preparation/cleanup remnant; malformed existing journals still fence. Tests cover both preparation and partial post-commit cleanup remnants across restart.

await platform.installPackage(await writeFixturePackage(root, 'recover-package', 'recover'));
await platform.close();
const packages = join(control, 'plugin-packages-v2');
await rename(join(packages, 'recover-package'), join(packages, '.previous-owner-death'));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] This renames a package to a top-level .previous-owner-death, but prepareInstall creates previous at join(transaction, 'previous') — inside the .install-<uuid> directory. No production path writes a top-level .previous-*, so this tests a state owner death cannot leave behind, and its only real effect is keeping the unreachable .previous- branch alive.

The larger point: nothing in the suite ever creates an .install-<uuid>/ with a transaction.json, so #recoverInstall's three-way generation arbitration — the crash-consistency authority of this subsystem — has zero coverage. Both P1s I filed on plugin-package-store.ts live there.

Relatedly, all four fault-injection stores override replace(), so no test makes a syscall fail. Moving published = true one line earlier in HostPluginCompositionStore.replace keeps the suite green while inverting the commit_outcome_unknown classification the whole fence rests on.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5071441. I removed the synthetic top-level .previous-* test and replaced it with real .install-*/transaction.json states. The matrix covers candidate/target/previous/rejected at every publish and rollback rename boundary, authority at base and next generation, fresh install versus replacement, missing journal cleanup, and corrupt journal fencing.

const next = compositionAuthority(
planned.generation,
this.#authority.packageLayers,
Object.freeze([...this.#authority.overlays, ...normalizedInput.operations]),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Overlays are append-only and nothing ever shortens them — an insert and its later removal both live here forever. Once the log crosses 4096 operations or the 2 MiB file cap, store.replace throws and apply reports persistence_failed with "Runtime state was not changed": accurate, permanently true, and pointing at the wrong cause. Every later apply fails identically and recovery means hand-editing plugin-composition-v2.json. Startup also replays the whole log every time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that the bounded append-only overlay log eventually exhausts its authority file. I am not adding compaction/reset semantics in this foundation PR: that would introduce a second authority rewrite policy (including generation, audit, and crash-recovery semantics) rather than repair an incorrect implementation of the current package-layer + user-overlay model. The current limits remain explicit and fail closed. I will treat compaction as follow-up work when the first composition consumer establishes the required retention/audit contract, rather than enlarge this already broad foundation change.

authority: PersistedPluginComposition,
): Promise<MakaCompositionState> {
let working = emptyCompositionState();
for (const extensionId of authority.packageLayers) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] This folds packageLayers + overlays the same way #composeLayers does, but with different rules: #composeLayers normalizes configuration per layer inside the fold and then validates, while this applies every operation raw and normalizes the whole tree afterwards, with no validation. The desired tree after a restart is therefore derived by a different code path than the one that accepted it.

Recovery does need to be fail-open and to adopt the durable generation, but those are parameters, not a second algorithm — one fold taking {validate, generation} covers both and removes the ordering difference.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not find a normally committed authority state for which these paths produce different Entry trees. The distinction that remains is intentional: online mutation normalizes and validates before committing; startup must adopt the already durable generation fail-open and let Runtime recovery isolate broken Entries instead of retroactively rejecting Host startup. Unifying them behind a validate flag would be a refactor, not a demonstrated P2 fix, and would add churn to this foundation PR. I am leaving the two paths in place unless there is a concrete reachable counterexample.

);
}
if (planned) {
await this.#replaceDesiredComposition(planned, packageLayers, this.#authority.overlays);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] The package_in_use checks correctly moved ahead of this line, but the failure path after it did not. #replaceDesiredComposition durably commits the authority without the package layer; if packages.uninstall then fails with a non-ENOENT rename error it throws persistence_failed, and the catch restores the runtime without ever restoring the authority.

The result is durable authority saying "uninstalled", bytes still on disk, runtime still holding the package, and the caller told it failed — and after restart the patch is never replayed, so the package silently disappears.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed and fixed in dd5eec8. A known-not-committed Package-store uninstall failure now restores the prior package generation, durable packageLayers/overlays authority, desired Entry tree, and live Runtime state. The restoration publishes a fresh monotonic generation; if any rollback step fails, the Platform fences and reports an AggregateError containing both the uninstall and rollback failures. The new fault-injection test verifies stored authority, desired roots, active Runtime Entry, and Package bytes all remain present.

const replacement = await this.#stage(
serialize(current),
current.rootId,
current.parent,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] #reloadPackage stages the new mount before disposing the previous one, and the same make-before-break shape is in #rebind, #replace, and #replaceComposition. Because PluginContext#label() falls back to a kernel-global symbol and provide() rejects a label already in #kernel.services, the staged Fiber's ctx.provide throws while the old Fiber still holds it — so move and reload fail for any Entry that provides a service.

isolate is not a workaround: it gives the provider a fresh private symbol each stage, so consumers looking up the global one no longer find it. No test covers this because every provide in the suite happens on loader.root or an external Context, never on a plugin entry.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The make-before-break/global-service collision is a real limitation of the existing Composition Loader/Service kernel from #3250, but it is not introduced by the package-foundation authority in this PR. Fixing it requires choosing transactional service publication or changing global service ownership across reload/rebind/replace—not a local Package lifecycle repair—and would redesign the already-merged kernel contract. I am keeping this PR scoped to the generic Package/Composition foundation and will handle service hot-swap with the first Service consumer/kernel follow-up.

// that case would unnecessarily dispose the current Fiber and lose
// its registered contributions.
if (appliedOperations > 0) await this.#replaceSnapshot(before, 'rollback');
if (appliedOperations > 0) await this.#replaceComposition(before, 'rollback');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Two things here.

before is the full compositionState() — profile, desktop-ui, and every session:* — so a batch that fails after one applied operation stops and restarts every plugin Fiber in every unrelated session, losing in-memory state and re-registering contributions. That contradicts the comment just above, and the suite treats "does not restart unrelated Entries" as a correctness requirement for reload at :714.

Second: if #replaceComposition itself throws, #roots/#entries keep the half-applied state, the generation counter is not advanced, and throw error on the next line is never reached — so the original failure reason is replaced by the rollback error. An AggregateError([error, rollbackError]) would at least preserve it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split the two observations. Commit dd5eec8 fixes the concrete error-loss bug: if full-composition rollback fails after a partially applied batch, the caller now receives AggregateError([originalError, rollbackError]), with a regression test proving both causes survive. I am not replacing the existing full-state compensation with a targeted multi-root transaction here. apply batch rollback predates this PR and has a different contract from targeted package reload; preserving unrelated in-memory Fiber state would require a new transactional Loader design, not a small foundation fix.

for (let index = cursor; index < values.length && items.length < limit; index += 1) {
const candidate = [...items, values[index] as T];
if (
Buffer.byteLength(JSON.stringify({ view, items: candidate, nextCursor: index + 1 }), 'utf8') >

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] This 480 KiB budget and MAX_FRAME_BYTES = 512 * 1024 in protocol/plugin-platform.ts:55 are one budget written twice, with a 32 KiB implicit headroom, and the two measure different objects (pre-encode structure here, decoded result there). Today the headroom covers it; changing either alone will not be caught, since no query test ever produces more than one item.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0239008. The coordinator and protocol decoder now share the exported PLUGIN_PLATFORM_QUERY_RESULT_MAX_BYTES constant, so the producer and consumer enforce one result-object budget. A regression test creates twelve roughly 60 KiB Entry projections, verifies that one page contains multiple items but stops before the shared limit, retains nextCursor, and round-trips through decodeResponseFrame.

@github-actions github-actions Bot added the effort/XL Over 1000 readable lines label Aug 27, 2026
@xxhZs

xxhZs commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up on the three prototype-key P3s from the review body: fixed in 0239008. (1) applyCompositionState now creates a missing session scope with an own data property, so session:proto is preserved without invoking Object.prototype.proto or throwing from siblings.splice. (2) scalar/isolate decoders build records with Object.fromEntries; valid prototype-named scalar keys survive, while an invalid proto service key is now rejected rather than silently disappearing. (3) Extension configuration reads only own input properties, so declared constructor/toString/valueOf-style keys no longer resolve inherited values. Focused reducer, protocol, and manifest tests were added. Runtime: 3051 tests, 0 failures (13 skipped). Runtime Host: 1207/1207. Biome, ASF headers, and diff check pass.

@likun666661

Copy link
Copy Markdown
Member

State-machine review: make the hidden product state explicit

I reviewed the current head (023900813) as a state-coordination protocol rather than as a collection of Plugin APIs. The package transaction fixes on this head materially improve the crash boundaries. The remaining concern is now one level higher: several correctness properties depend on state that exists only implicitly in call order, private booleans, temporary journal layout, or operator knowledge.

The plain-language model

Imagine the Host has four things:

  1. A locked cupboard of Plugin boxes — the Package Store. Each box contains code, a manifest, and possibly a Composition patch.
  2. An official ledger — the persisted Composition authority. It records an epoch, the ordered Package layers, and the User overlay operations.
  3. A written plan reconstructed from the cupboard and ledger — the Desired Entry Tree.
  4. The machines currently running in the house — the live Context/Fiber Runtime.

The important rule is that these are not the same state:

P = Package bytes, manifests, patches, and Runtime modules
A = persisted authority: epoch + packageLayers + overlays
D = derive(P, A): the Desired Entry Tree
L = converge(D, code(P)): the live Fiber Tree
Q = inspect(L): the operator-facing projection

(P, A) -> D -> L -> Q

The official facts are jointly P + A. D is an in-memory projection. L is a fallible execution projection. Q is only an observation of that execution projection.

This distinction explains the otherwise unusual mutation order:

install:   publish Package bytes -> commit authority -> converge Runtime
uninstall: commit authority      -> detach Runtime  -> remove Package bytes
apply:     commit authority      -> converge Runtime

Installation must put the box in the cupboard before the ledger is allowed to reference it. Uninstallation must remove the ledger reference before throwing the box away. Extra, unreferenced bytes are a safe orphan; an authority that references missing bytes is not safe.

Package replacement adds a recoverable receipt:

prepare candidate
  -> fsync candidate
  -> journal baseEpoch -> nextEpoch
  -> publish candidate as canonical Package
  -> commit Composition authority at nextEpoch
  -> remove transaction remnants
  -> converge live Runtime

After owner death, the persisted authority is the transaction decision record:

authority == baseEpoch  -> restore the previous Package
authority >= nextEpoch  -> retain the candidate Package
anything else           -> the evidence is ambiguous; fence

That ordering is the strongest part of this PR. The Plugin Platform is effectively a transaction coordinator for two durable stores followed by a Runtime reconciler.

This is a product of state machines, not one state machine

At minimum, the implementation has these independent axes:

Control phase:
  NEW | RECOVERING | READY | FENCED | DRAINING | CLOSED

Convergence:
  UNKNOWN | CONVERGED | DIVERGED

Package transaction:
  ABSENT | PREPARED | PUBLISHED_UNCOMMITTED | COMMITTED | ROLLING_BACK

Authority commit:
  OLD | COMMITTING | COMMITTED | OUTCOME_UNKNOWN

Live Runtime:
  OLD | STAGING | CONVERGED | PARTIALLY_RECOVERED | FAILED

Today those states are encoded indirectly by #closed, #draining, #poisoned, #diverged, filesystem layout, authority generation, and whether a particular method has already been called. Only a subset of the possible combinations is valid, but the type/API boundary does not say which subset.

The state-machine review question is therefore not merely “does each method work?” It is:

For every externally observable transition, can the implementation prove from durable evidence whether the command was rejected, committed, committed-but-not-converged, or ambiguous — and can the caller observe that distinction without guessing?

On the current head, several answers still depend on hidden state.

Blocking implicit state dependencies

1. recover() must happen exactly once, but this lifecycle state is not represented

HostPluginPlatform starts with an empty in-memory authority and immediately permits installPackage, uninstallPackage, apply, and read as long as it is not closed, draining, or poisoned. There is no NEW/READY admission check.

Production composition currently calls recovery before exposing ready operations, but the class is public and the invariant is not enforced by the class. Calling a mutation before recover() can plan from generation 0 and an empty layer list even when durable authority already exists.

The inverse transition is also unguarded: calling recover() a second time on a live instance runs packageLoader.collectGarbage() before rebuilding. That can remove executable-generation directories still owned by live Fibers. The review response says recovery is intentionally one-shot; the state machine should encode and reject the unsupported transition rather than rely on caller knowledge.

Required model:

NEW -> RECOVERING -> READY | FENCED
READY -/-> RECOVERING
NEW -/-> mutation/query

2. A known durable commit is erased at the protocol boundary

HostPluginPlatform.apply() and installPackage() correctly distinguish a post-commit Runtime failure internally. They set #diverged and throw HostPluginPlatformError('mutation_failed', '... authority was committed but Runtime convergence failed').

HostPluginPlatformCoordinator.failure(), however, unwraps every mutation_failed to its cause:

if (error.code === 'mutation_failed' && error.cause) return failure(error.cause);

The wire caller can therefore receive an ordinary internal_failure or another cause-derived error for two fundamentally different outcomes:

A. rejected before commit; durable state did not change
B. authority committed; Runtime did not converge

Retrying B as though it were A is unsafe. A retry can append another overlay, hit entry_exists/entry_not_found, or turn an install acknowledgement retry into a replacement.

This is not commit_outcome_unknown; the outcome is known to be committed. It needs a first-class wire result, for example:

{ ok: true, result: { generation, convergence: 'diverged', failures } }

or a distinct committed_runtime_failed outcome carrying the committed generation. The coordinator must not discard the commit fact.

3. #diverged changes the transition algorithm but is not observable

#diverged is not a diagnostic-only bit. It decides whether a later apply uses incremental composition.apply() or full recoverComposition(), and whether reload triggers reconciliation. It is therefore part of the product state machine.

The status query does not expose it. Instead it combines:

generation   <- Desired authority
packageCount <- physical Package Store
entryCount   <- live Runtime
failureCount <- ephemeral in-memory failure projection

During divergence, status may report the new authority generation, the old live Entry count, and zero failures. In particular, a replacement can fail during composition.reload() after authority commit but before #publishEntryFailures() records Entry failures.

Status should expose at least:

{
  phase,
  authorityEpoch,
  convergence: 'converged' | 'diverged',
  desiredEntryCount,
  liveEntryCount,
  installedPackageCount,
  layeredPackageCount,
}

There is also no active reconciliation loop. Divergence is repaired only by a later mutation/reload or by restart, so an idle Platform can remain diverged indefinitely. That trigger dependency must be explicit, or a plugin.platform.reconcile operation/background policy must own it.

4. The authority is composite, but no durable Package revision is recorded

The persisted authority stores Package IDs, not Package revisions:

{ generation, packageLayers: string[], overlays }

The Desired Tree is not determined by that file alone. It also depends on the current manifest and patch bytes under each canonical Package directory. The install journal temporarily binds “old bytes” and “candidate bytes” to baseGeneration -> nextGeneration, but after transaction cleanup the authority retains no version, digest, or patch identity.

Therefore the real durable authority is (Composition file, canonical Package bytes), under the implicit assumption that no path except PluginPackageStore mutates those bytes.

That may be the intended trusted-control-directory contract, but it should be explicit and inspectable. A stronger authority would reference an immutable revision/digest:

{ extensionId, packageRevision, layerPosition }

The Package query should expose the revision as well. Otherwise an operator can identify the Package ID but cannot prove which code/patch revision produced the current Desired Tree.

5. Serialization is part of correctness, but bypass paths are public

Between Package publication and authority commit, canonical Package bytes and authority intentionally disagree. The design is safe only because:

  • every query and mutation uses the same serialized queue;
  • recovery runs before the Host serves requests;
  • no other writer mutates either store;
  • no caller reads the canonical Package directly during the transition.

Those are transaction invariants, not implementation conveniences.

However PluginPackageStore, HostPluginCompositionStore, and their direct mutation methods are exported. PluginPackageStore.install() can publish with hard-coded 0 -> 1 without coordinating Composition authority. A caller can therefore bypass the only coordinator that makes the two stores consistent.

If HostPluginPlatform is the sole coordinator, lower-level mutation capabilities should be internal or explicitly marked unsafe/test-only. Read-only inspection can remain public; cross-authority writes should not.

6. Package patches create an undeclared dependency graph

The manifest declares explicit Package dependencies, and the Platform validates their presence, cycles, same-root activation, and uninstall protection.

But ordered Composition patches create a second dependency channel. Package B can update, move, disable, or remove an Entry introduced by Package A. B's patch can only replay if A's Entry already exists, so B has a structural dependency on A even when B's manifest declares none.

This hidden edge affects:

  • installation order;
  • replacement validation;
  • startup replay;
  • uninstall planning;
  • operator explanations.

For example, uninstalling A can make replay of B fail on entry_not_found, which means A is effectively in use by B, but Package query exposes only manifest dependencies.

The model needs one of these explicit contracts:

  1. patch references generate dependency edges and become queryable;
  2. a Package patch may mutate only Entries owned by that Package;
  3. cross-Package overrides require an explicit extends/overrides declaration.

Without one, packageLayers is simultaneously precedence order and an invisible dependency graph.

7. Online derivation and recovery derivation are two algorithms

Online planning uses #composeLayers(): normalize each layer, apply it, normalize overlays, then validate strictly before commit.

Startup uses #composePersistedAuthority(): replay raw layers and overlays, normalize the completed tree, then isolate invalid Entries during recovery.

The strict-versus-fail-open policy difference is legitimate. The implicit dependency is that both implementations must forever derive the same valid tree despite different ordering and validation paths.

This should be one projector with explicit policy:

deriveDesired({
  authority,
  packageResolver,
  validation: 'strict' | 'isolate-invalid',
}) -> { desired, failures, dependencyGraph }

Then online mutation and startup recovery differ by policy rather than by duplicated causal algorithms.

8. Three unrelated concepts are all named generation

The system currently has:

  1. Composition authority generation — the durable transaction epoch;
  2. Fiber generation — the activation instance shown in Entry inspection;
  3. executable Package generation — the immutable UUID directory holding code.

They are not comparable, but they share the same word in the same protocol/domain. baseGeneration/nextGeneration refer to authority; Entry generation refers to Fiber activation; plugin-generations-v1 refers to code revision.

These should be distinct types and names, such as:

authorityEpoch
fiberActivationId
packageRevision

Otherwise future code will eventually compare or report the wrong generation.

9. Paged queries depend on a stable snapshot that is not represented

The cursor is an array offset. Each page is serialized, but separate page requests can have mutations between them. Packages, live Entries, and failures can be inserted, removed, or reordered, causing an offset cursor to skip or repeat items.

The cursor needs to bind to a projection revision/snapshot token, or the request must carry an expected authority/live projection epoch and reject when it changes.

10. The contribution state machine is not connected in production yet

Production constructs new HostPluginPlatform(controlDirectory), which creates a default MakaCompositionLoader() with no Runtime Host root Context and no concrete Tool/UI/Hook/Event registry transaction. MakaPluginPackage.contributions is validated and retained, but there is no product contribution consumer in this PR.

The implemented causal chain therefore currently ends at:

Package -> Desired Entry -> Context/Fiber

not yet:

Package -> Tool/UI/Hook/Event contribution -> product behavior

That missing edge matters to state correctness. Make-before-break reload currently stages a new Fiber while the old Fiber still owns global Service labels; a Service-providing Plugin can collide before the old owner retires. Contribution publication, rollback, partial recovery, and external registry ownership need at least one concrete consumer test before the Platform can claim end-to-end convergence semantics.

State invariants I would make executable

The following should be named invariants with operation-boundary and crash-boundary tests:

I1. READY implies recovery completed exactly once.
I2. authorityEpoch == desired.authorityEpoch in every settled in-memory state.
I3. derive(P, A) == D, or the Platform is FENCED with durable evidence explaining why.
I4. L matches D, or convergence == DIVERGED and that state is externally observable.
I5. A reference to a Package implies the referenced Package revision exists.
I6. A pre-commit rejection is distinguishable from a committed Runtime failure.
I7. An unknown durable outcome fences every later mutation at dequeue time.
I8. Every dependency that constrains replay/uninstall is present in one queryable graph.
I9. Every public page belongs to a stable projection snapshot.
I10. No public lower-level write can bypass the cross-store coordinator.

Bottom line

The crash-recovery mechanics are now close to coherent. The main remaining risk is that the control plane does not yet expose the same state distinctions that the implementation relies on internally.

The most important fixes before merge are:

  1. preserve “committed but Runtime diverged” at the wire boundary;
  2. expose convergence and desired/live projections in status;
  3. encode NEW -> RECOVERING -> READY/FENCED and reject unsupported lifecycle transitions;
  4. bind authority to an inspectable Package revision;
  5. remove public cross-store write bypasses;
  6. make patch-reference dependencies explicit;
  7. unify Desired derivation behind one policy-driven projector.

Once those states and dependencies are explicit, the Package journal, authority commit, and Runtime reconciliation can be reasoned about as one defensible system rather than as a set of methods whose correctness depends on callers already knowing the hidden state machine.

@xxhZs
xxhZs force-pushed the feat/plugin-platform-foundation branch from 0239008 to 39937b5 Compare August 29, 2026 11:26
@xxhZs

xxhZs commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

Implemented the state-machine review on the rebased head 39937b5d7 (base 8b93dd52b). This supersedes my earlier scope objections on the affected threads.

  1. Recovery lifecycle: HostPluginPlatform now enforces one-shot recovery, rejects mutation/query before readiness, and keeps recovery completion independent from drain/close so production shutdown remains safe.
  2. Known commit outcome: post-commit Runtime failures return a committed mutation receipt with authorityEpoch and convergence: diverged; the coordinator no longer unwraps away the commit fact.
  3. Observable convergence: status exposes phase, authority epoch, convergence, physical/layered Package counts, desired/live Entry counts, failures, and a bounded fence diagnostic. Divergence gets both explicit reconcile and a bounded backoff reconciler.
  4. Package identity without restoring Revision: the authority remains canonical Package bytes + Package IDs/layers + overlays. Package query exposes a computed canonical contentDigest for inspection only; no Package Revision/Version/Binding state was reintroduced.
  5. Single coordination boundary: direct Store mutation is removed and Store/Loader implementations are no longer public server exports; the Platform privately owns all cross-Store transitions.
  6. Cross-Package patches: manifests explicitly declare composition.structuralDependencies; the shared projector verifies the actual cross-Package structural edges, query exposes them and reverse requiredBy, and uninstall protects both manifest and structural dependents.
  7. One Desired projector: online planning and recovery now use the same derivation implementation with strict versus recovery policy parameters.
  8. Stable pagination: cursors are opaque and snapshot-bound (view/root/digest/offset); a mutation between pages produces stale_cursor rather than silent duplicates/omissions.
  9. Generation terminology: public Platform receipts/status use authorityEpoch. Computed Package identity is contentDigest; Fiber inspection keeps its nested Runtime generation. The composition baseGeneration field remains only the existing CAS input.
  10. Real consumer boundary: the PR description now says Package -> Desired Entry -> Context/Fiber, not generic contribution convergence. No Tool/UI/Hook/Event consumer was added. The concrete single-provider Service collision is fixed with break-before-make Package replacement and covered by a real provider regression.

Also changed Plugin Platform admission to local-owner-only, made committed uninstall cleanup failure caller-visible as cleanup: pending, and added loaded-generation release on pre-adoption failures.

Verification on the pushed head: complete workspace build; Runtime Host 1390 passed / 9 skipped / 0 failed; focused Plugin Platform 35/35; focused CLI 7/7; Biome and git diff --check pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants